-
-
Notifications
You must be signed in to change notification settings - Fork 763
feat: add --trust CLI and remote.trust config for remote tasks #2491
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Maciej Lech <[email protected]>
|
Looking forward to this feature! |
| // Extract host from server URL for trust testing | ||
| parsedURL, err := url.Parse(srv.URL) | ||
| require.NoError(t, err) | ||
| trustedHost := parsedURL.Host |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't I trust URLs? I trust github.com/myself but I don't trust github.com/shady.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking about that, but using URLs requires more assumptions how to compare provided URL with the trust config.
- Exact match, so full URL comparison. In my case it would require configuring every single remote taskfiles (more than dozen now) which is not a big deal, but may not be a best DX.
- Prefix match. A problem: I want to trust
https://github.com/myselfbut nothttps://github.com/myselfHackedByShady- which could be easily solved by settinghttps://github.com/myself/and nothttps://github.com/myself. So maybe this is the best way. - Glob-like style:
https://github.com/myself/*or extended versionhttps://github.com/myself/**/* - Regex:
https:\/\/github\.com\/myself\/.*
vmaerten
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your PR!
I would prefer using trusted-hosts instead of trust (both in the CLI and in the config file).
- It's more explicit, even if it's longer
- Tools like pip already use this flag (see: https://pip.pypa.io/en/stable/cli/pip/#cmdoption-trusted-host)
- Using the CLI with comma-separated values (
--trusted-hosts a.com,b.com) avoids repeating the flag name
| // Merge Trust lists - combine both lists with other's entries taking precedence | ||
| // Remove duplicates by using a map | ||
| if len(other.Remote.Trust) > 0 { | ||
| seen := make(map[string]bool) | ||
| merged := []string{} | ||
|
|
||
| // Add other's hosts first | ||
| for _, host := range other.Remote.Trust { | ||
| if !seen[host] { | ||
| seen[host] = true | ||
| merged = append(merged, host) | ||
| } | ||
| } | ||
|
|
||
| // Then add base's hosts that aren't duplicates | ||
| for _, host := range t.Remote.Trust { | ||
| if !seen[host] { | ||
| seen[host] = true | ||
| merged = append(merged, host) | ||
| } | ||
| } | ||
|
|
||
| t.Remote.Trust = merged | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the order doesn’t matter (a host is trusted regardless of its position in the slice), we can simplify it to:
| // Merge Trust lists - combine both lists with other's entries taking precedence | |
| // Remove duplicates by using a map | |
| if len(other.Remote.Trust) > 0 { | |
| seen := make(map[string]bool) | |
| merged := []string{} | |
| // Add other's hosts first | |
| for _, host := range other.Remote.Trust { | |
| if !seen[host] { | |
| seen[host] = true | |
| merged = append(merged, host) | |
| } | |
| } | |
| // Then add base's hosts that aren't duplicates | |
| for _, host := range t.Remote.Trust { | |
| if !seen[host] { | |
| seen[host] = true | |
| merged = append(merged, host) | |
| } | |
| } | |
| t.Remote.Trust = merged | |
| } | |
| if len(other.Remote.Trust) > 0 { | |
| merged := slices.Concat(other.Remote.Trust, t.Remote.Trust) | |
| slices.Sort(merged) | |
| t.Remote.Trust = slices.Compact(merged) | |
| } | |
| task --trust github.com --trust gitlab.com -t https://github.com/user/repo.git//Taskfile.yml | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| task --trust github.com --trust gitlab.com -t https://github.com/user/repo.git//Taskfile.yml | |
| task --trust github.com --trust gitlab.com -t https://github.com/user/repo.git//Taskfile.yml | |
| task --trust github.com,gitlab.com -t https://github.com/user/repo.git//Taskfile.yml |
This PR implements the feature discussed in #2473.
It introduces a new --trust CLI flag and a corresponding remote.trust configuration option in taskrc. These options are available when the Remote Taskfiles experiment is enabled.
The --trust flag can be specified multiple times to define trusted hosts (optionally including ports). Any remote Taskfiles fetched from these trusted hosts will not prompt for confirmation on their initial download or when their checksums change.
Closes #2473